Skip to content

Fix fread segfault with \x1a bytes #7415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix fread segfault with \x1a bytes #7415

wants to merge 2 commits into from
+11 −0

Conversation

@ANAMASGARD
Copy link

@ANAMASGARD ANAMASGARD commented Nov 5, 2025

What I Added

Two test cases that show the bug exists:

  • Test 9063.1: Long field ending with \x1a
  • Test 9063.2: Short CSV with \x1a at end

Both tests crash right now (expected).

What Comes Next

Next commit will fix the actual bug in src/fread.c by handling \x1a as a field terminator.

Screenshots

Screenshot From 2025-11-05 16-41-27

See attachment: Screenshot showing the segfault crash message

Fixes #7407

@ANAMASGARD
Add test for fread segfault with \x1a bytes
d947244 
Reproduces issue Rdatatable#7407 where fread crashes when reading files
with \x1a (ASCII SUB) character. Manual testing confirms both
test cases trigger segmentation fault as expected.
@MichaelChirico
Copy link
Member

MichaelChirico commented Nov 5, 2025

Please confirm you've read CONTRIBUTING

@ANAMASGARD
Copy link
Author

ANAMASGARD commented Nov 6, 2025 •
edited
Loading

Yes @MichaelChirico I've read the CONTRIBUTING guide. This PR adds test cases for #7407.
My approach is to locally try to reproduce the issue first and push it as the first commit in the PR ,
It fails which means the issue is there and in the next commit will include the actual fix for src/fread.c.

I think in the issue section I should have first explain my approach regarding how I am going to fix the issue like a discussion before raising PR ,
Also I honestly I do use Perplexity to understand something when i get stuck but i don't blindly copy and paste code , I thoroughly run test and verify that if the issue is fixed on not before commiting it .

This is not written by AI it's me only .

@MichaelChirico
Copy link
Member

MichaelChirico commented Nov 6, 2025

@ANAMASGARD please read it very carefully, especially about how to use test().

I'm also not sure what to do with a PR that only adds a failing test. Could you please mark it as Draft if you plan to also fix the issue, so that it's out of the review queue in the meantime?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaelChirico MichaelChirico Awaiting requested review from MichaelChirico MichaelChirico is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fread() allergic to \x1a bytes

2 participants

@ANAMASGARD @MichaelChirico